-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/gql exporter #433
base: master
Are you sure you want to change the base?
Feature/gql exporter #433
Conversation
8e6ef04
to
dcde474
Compare
field_dict[additional_field[0]] = field(node, f" {str(additional_field[1])}: ") | ||
def get_gql_unit_enums() -> Dict[str, graphene.Enum]: | ||
"""Get GraphQL enums for VSS units and quantity kinds.""" | ||
global mapping_quantity_kinds_df |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does this need to be global and not just an argument / return value of the function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For simplicity, to avoid passing that to many places. This variable is only modified once but accessed elsewhere
src/vss_tools/exporters/graphql.py
Outdated
|
||
def get_graphql_schema(tree: VSSNode) -> graphene.Schema: | ||
"""Create a GraphQL schema from the VSS tree.""" | ||
global vss_metadata_df, vss_branches_df, vss_leaves_df, gql_unit_enums, gql_allowed_enums, gql_instance_enums |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So many globals. Isn't it possible to pass them through where needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible, but as I have many functions, it will imply passing them over multiple times.
5190be1
to
50cadb6
Compare
MoM:
|
): | ||
""" | ||
Export as GraphQL. | ||
Export a VSS specification to a GraphQL schema. | ||
""" | ||
tree, _ = get_trees( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about the type tree aka structs?
50cadb6
to
a5728b5
Compare
dc8acfa
to
142797e
Compare
ee4dcf1
to
9942f6c
Compare
MoM:
|
Signed-off-by: Daniel Alvarez-Coello <[email protected]>
9942f6c
to
63095a8
Compare
There are other features or possible improvements to the GraphQL VSS counterpart. For example:
I am happy to hear more feedback from the community in case there is any suggestion or observation about the modeling decisions and proposed structures to make this even better. |
MoM:
|
This PR addresses the issue #431. It refactors entirely the GraphphQL exporter to offer a usable schema structure. For extended explanation, please see the updated documentation of the exporter
docs/graphql.md
.